Fix issue 563#1958
Open
KyWB wants to merge 3 commits into
Open
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion#563) - Extract shared resolvePubkeyOrOns() helper - ModeratorsAddDialog: async ONS resolution before submit, inline errors, spinner - OverlayMessage: refactored onto shared resolver (~45 lines collapsed to one call) - Add 11 unit tests covering all resolver paths
Author
|
Hi! This is my first contribution to Session Desktop. I've addressed issue #563 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First time contributor checklist:
Contributor checklist:
devbranchyarn readyrun passes successfully (more about tests here)Description
Fixes #563 — ONS names entered into the Add Moderator/Admin dialog were
immediately rejected with "This Account ID is invalid" instead of being
resolved to their corresponding 66-character hex public key.
Root cause:
ModeratorsAddDialog.tsxvalidated input directly viaPubKey.from()with no prior ONS resolution step. The ONS resolver(
ONSResolve.getSessionIDForOnsName) already existed and was used by theNew Message flow (
OverlayMessage.tsx), but was never called from themoderator dialog.
What this PR does:
resolvePubkeyOrOns()ints/session/utils/resolvePubkeyOrOns.tsthat encapsulates the fullresolution ladder: trim/normalize -> pass through valid non-blinded
05pubkeys -> reject blinded/
03-group keys -> ONS regex gate -> networkresolve -> re-validate. Returns a discriminated union and never throws.
ModeratorsAddDialog.tsxto resolve each comma-separated entrythrough the shared helper before submission. Shows a spinner and disables
the Add button during resolution. Surfaces failures inline via
providedErrorrather than only a generic toast. Multi-add inputs namethe specific failing entry.
OverlayMessage.tsx(New Message flow) onto the same sharedhelper, collapsing ~45 lines of inline resolution logic into a single
call. Behavior is fully preserved.
accountIdtoaccountIdOrOnsEnterso the dialog advertises ONS support.Test approach:
ts/test/session/unit/utils/resolvePubkeyOrOns_test.tscovering: valid pubkey passthrough, whitespace trim, blinded key rejection,
03-group key rejection, empty input, successful ONS resolution, dotted-name
rejection, NotFoundError mapping, SnodeResponseError mapping, generic error
mapping, and resolved-but-invalid-id handling.
SESSION_DEV=1: entering a valid ONSname in Add Admins now triggers async resolution; valid hex pubkeys still
work unchanged; invalid inputs still show appropriate errors.